Add swift-configuration support #878
Conversation
Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift
Show resolved
Hide resolved
| public init(configReader: ConfigReader) throws { | ||
| self.init() | ||
|
|
||
| // Each entry in the list should be a colon separated pair e.g. localhost:127.0.0.1 or localhost:::1 |
There was a problem hiding this comment.
colon felt more natural to me for defining key-value pairs. I don't mind though
There was a problem hiding this comment.
comma would probably make the ipv6 case slightly eaiser to parse too
There was a problem hiding this comment.
Oh right, comma might be used for splitting the pairs themselves e.g. in env vars. Hmm then comma isn't great, but colon is a bit confusing.
|
lgtm, I'll let the maintainers actually approve though 🙂 |
| /// - `dnsOverrides` (string array, optional): Colon-separated host:IP pairs for DNS overrides (e.g., "localhost:127.0.0.1"). | ||
| /// - `redirect` (scoped): Redirect handling configuration read by ``RedirectConfiguration/init(configReader:)``. | ||
| /// - `timeout` (scoped): Timeout configuration read by ``Timeout/init(configReader:)``. | ||
| /// - `connectionPool` (scoped): Connection pool configuration read by ``ConnectionPool/init(configReader:)``. | ||
| /// - `httpVersion` (string, optional, default: automatic): HTTP version to use ( "automatic" or "http1Only"). | ||
| /// - `maximumUsesPerConnection` (int, optional, default: nil, no limit): Maximum uses per connection. |
There was a problem hiding this comment.
Should we do camelcase or dot separated? In Temporal we choose the latter https://github.com/apple/swift-temporal-sdk/blob/eb356f25b53d4ac50b1505dea1b7306ba7296bc8/Sources/Temporal/Client/TemporalClient%2BConfiguration.swift#L111
There was a problem hiding this comment.
I used dots for hierarchy but camel case for words
There was a problem hiding this comment.
dns.overrides and http.version make sense in that system. But maximum.uses.per.connection does not
There was a problem hiding this comment.
This could be connection.limits.maximum.uses. It is not the end of the world but it would be great if we could be a bit consistent around the ecosystem. @czechboy0 WDYT?
There was a problem hiding this comment.
oh yeah i like that idea
There was a problem hiding this comment.
I've been doing the same as Hamzah - use dots for separating components, with individual components being camelCase. So http.maxConnectionCount, for example.
3 participants
Users should be able to use swift-configuration to create the http client configuration object
Changes